Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port to TypeScript + 0.10.0 Gamemode Changes #146

Merged
merged 38 commits into from
Sep 8, 2024

Conversation

tsa96
Copy link
Member

@tsa96 tsa96 commented Aug 28, 2024

Working on Pano seriously for the first time in ages doing gamemode changes and was very unhappy with the quality of the TypeScript integration, mainly the namespacing issues caused by not being able to use imports. I eventually snapped and adding module loading support in C++, then found I was now liking the TypeScript integration so much I ended up porting everything across.

There were enough bugs, random unused events/APIs and other issues I encountered along the way that it was definitely justified. Would've preferred doing after 0.10.0 but hey, it meant we can get the shared types/enums from the website repo hooked up whilst doing all the updated timer, comparisons etc.

Momentum-specific types have been split out to this repo as discussed. I've almost upgraded all our deps, had to mess around with eslint in the process.

The one thing I haven't quite finished yet is the map selector. New modes are hooked up but I want to get zone downloading working by Saturday, maybe even do the submission maps tab if I have time. Everything else is good for review, if people reach the wip map selector stuff fast then I'll split that work out into a separate PR, but I expect it'll take at least a week for this work + all my other stuff to get reviewed.

Needs StrataSource/pano-typed#22 on the shared types repo, then on red, my module loading PR and gamemode enum PR.

Checks

  • I have thoroughly tested all of the code I have modified/added/removed to ensure something else did not break
  • I have followed semantic commit messages e.g. feat: Add foo, chore: Update bar, etc...
  • My branch has a clear history of changes that can be easy to follow when being reviewed commit-by-commit
  • My branch is functionally complete; the only changes to be done will be those potentially requested in code review
  • Fully tokenized all my strings (no hardcoded English strings!!) and supplied bulk JSON strings below
  • All changes requested in review have been fixuped into my original commits.

@tsa96 tsa96 changed the base branch from master to feat/mom-0.10 August 28, 2024 03:42
@tsa96
Copy link
Member Author

tsa96 commented Aug 28, 2024

Will resolve conflicts with Peen's stuff once #145 is in (and I've finally slept)

@tsa96 tsa96 force-pushed the refactor/typescript-and-gamemodes branch from b7705e0 to cdb9f89 Compare August 28, 2024 03:47
Copy link
Member Author

@tsa96 tsa96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing my own work since this is such a chore for everyone else to do, plus afaik I'm the only Pano person who works with TypeScript frequently.

Others reviewing is by-all-means appreciated, probably just need one other though this is such a slog to review.

I've tested pretty well ingame so far, going to do a second pass of ingame testing today as well.

scripts/common/online.ts Outdated Show resolved Hide resolved
scripts/common/gamemode.ts Show resolved Hide resolved
scripts/components/chat.ts Outdated Show resolved Hide resolved
scripts/components/chat.ts Outdated Show resolved Hide resolved
scripts/hud/conc-cook-time.ts Outdated Show resolved Hide resolved
scripts/pages/zoning/zoning.ts Show resolved Hide resolved
scripts/util/toast-manager.ts Outdated Show resolved Hide resolved
import { PanelHandler } from 'util/module-helpers';
import { tupleToRgbaString } from 'util/colors';
import { SpeedometerColorType, SpeedometerType } from 'common/speedometer';
import { TimerState_OLD } from 'common/timer';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move to new timer today

scripts/pages/settings/speedometer.ts Show resolved Hide resolved
scripts/util/module-helpers.ts Outdated Show resolved Hide resolved
@tsa96 tsa96 force-pushed the refactor/typescript-and-gamemodes branch from cdb9f89 to 7cbf1e9 Compare August 28, 2024 23:01
@tsa96
Copy link
Member Author

tsa96 commented Aug 28, 2024

Still need to get all the new timer stuff hooked up. Panzer's timer changes don't work on this branch (aren't in TypeScript with new data structures), currently my stuff is just overwriting his recent commit.

I'll join them together properly soon, want to remove the old APIs in the process. Will also merge with Peen's new zoning stuff, just don't have energy tonight.

Not doing for ages, don't any reason to keep the stubbed stuff around.
Not pinning dev dependencies. It's not a big deal, and we're not in prod.
Top-level await yaaaay
"class-methods-use-this" is just annoying, not much benefit to static methods in JS,
and doesn't work well with our PanelHandler system
Idk, eslint updated and their config changed. Fun!
This causing people headaches, and not super beneficial.
Kiiiiind of works with debugger! And assume we'll want this once SCell's
thing is in.
@tsa96 tsa96 force-pushed the refactor/typescript-and-gamemodes branch 2 times, most recently from 3e71311 to c0408b7 Compare August 29, 2024 01:15
Copy link
Member

@Gocnak Gocnak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split out the map selector stuff but the rest is good 👍

@tsa96 tsa96 force-pushed the refactor/typescript-and-gamemodes branch from c0408b7 to b7e169b Compare August 31, 2024 03:54
@tsa96
Copy link
Member Author

tsa96 commented Aug 31, 2024

Release tonight will be pretty janky anyway so no need to rush this in, we can just cut release off this branch without merging.

@tsa96 tsa96 force-pushed the refactor/typescript-and-gamemodes branch 2 times, most recently from 60b1477 to 8615f32 Compare September 2, 2024 12:46
]
});

$.RegisterForUnhandledEvent('OnSynchroModeChanged', (cvarValue) => this.setDisplayMode(cvarValue));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question about registering events for game mode with the panel

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah same response, if config is always loaded on map init we can pull all events into the famcy thing, will do that.

There absolutely no code changes here, just moves, to keep files linked in Git and so review is easier.
Don't like having a dedicated script for something simple as this, better in <script> tag.
I know this feels a bit framework-y but with modules it's really needed,
otherwise exposing stuff to XML takes a lot of boilerplate.
Chungus commit but not that much to break up. Tried to not refactor too
much but I tweaked some things as I went. Couple of general comments:
- Suffixing every class with "Handler" is very verbose but omitting it
  will be a namespace collision for JS-defined panels like
  LevelIndicator, so figured I'd keep consistent and suffix everything.
- static is no longer required everywhere because of @PanelHandler
  instantiating classes it's applied to. Maybe a bit too magical, but I
  like not having static on absolutely everything.
- We should avoid .bind(this) with event handlers, it's easy to
  introduce type errors when using it, since it provides no type-safety
  between the event handler function signature and the function it's
  applied to. Arrow functions do the same thing, and make the arguments
  explicit.
This stuff is all interconnected so figured I'd split them out from main ts port commit
Refactored a bit of gamemode handling logic as well, needed a C++ fix to get config
loading working with new modes, that's in review on red.
Show/HideContentPanel weren't doing anything. ReloadBackground is ambigious. Guhh!!!
Not doing much styling in this PR, just couldn't resist improving these slightly.
This thing just confuses people, had multiple people ask why it doesn't work.
Doesn't look very good in its current state, may as well just hide for now.
Toop was having issues with default line endings in vscode, this should
set right defaults
@tsa96 tsa96 force-pushed the refactor/typescript-and-gamemodes branch from 2f49092 to 010d70c Compare September 7, 2024 21:05
@tsa96
Copy link
Member Author

tsa96 commented Sep 7, 2024

Okay I've addressed everything, good to merge once @PeenScreeker approves

scripts/util/math.ts Show resolved Hide resolved
scripts/util/math.ts Show resolved Hide resolved
}

export function sumOfSquares(vec: vec2 | vec3): number {
return 'z' in vec ? vec.x ** 2 + vec.y ** 2 + vec.z ** 2 : vec.x ** 2 + vec.y ** 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I wasn't super clear about this:
return 'z' in vec ? sumOfSquares3D(vec) : sumOfSquares2D(vec);

@PeenScreeker PeenScreeker merged commit d01849c into feat/mom-0.10 Sep 8, 2024
1 check passed
@PeenScreeker PeenScreeker deleted the refactor/typescript-and-gamemodes branch September 8, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants